================================================================================
REVIEW TO BE PUBLISHED
================================================================================

Verdict: COMMENT
PR: #4130 - [DASH] Enable dashboard dataset_catalog/schema in direct deployment mode

--- Overall Comment ---
## Issues Found

1. **Missing edge case test coverage**: The acceptance test only covers the happy path with both fields set to non-empty values. Consider adding tests for:
   - Empty strings for dataset_catalog/dataset_schema
   - Only one field set (catalog without schema, or vice versa)
   - Changing these fields during an update operation
   - Not setting these fields at all (to ensure backward compatibility)

2. **No unit tests for direct deployment dashboard resource**: The file `bundle/direct/dresources/dashboard.go` lacks unit tests. Consider adding tests to verify:
   - DoCreate properly passes dataset_catalog/dataset_schema to the API
   - DoUpdate properly passes dataset_catalog/dataset_schema to the API
   - DoRead correctly clears these fields (sets to empty string)
   - RemapState properly handles these fields

3. **FieldTriggers clarity**: Consider explicitly adding `dataset_catalog` and `dataset_schema` to the FieldTriggers map with `ActionTypeSkip` for both local and remote diffs. This would make the behavior more explicit and clear that these are request-only fields that should not trigger drift detection.

## Suggestions

1. The implementation correctly treats dataset_catalog and dataset_schema as request-only parameters by clearing them in DoRead and responseToState, which prevents false drift detection.

2. The acceptance test properly verifies that these fields are sent to the API via the captured POST request.

3. Consider adding a comment explaining why dataset_catalog and dataset_schema are always set to empty strings in DoRead and responseToState (i.e., they are request-only parameters not returned by the GET API).

--- Inline Comments (2) ---

[1] bundle/direct/dresources/dashboard.go:126
    Consider adding a comment here explaining why dataset_catalog and dataset_schema are always set to empty strings. These are request-only parameters and are not returned by the GET dashboard API.

[2] bundle/direct/dresources/dashboard.go:323
    Consider explicitly adding dataset_catalog and dataset_schema to the triggers map with ActionTypeSkip for clarity:

```go
triggers["dataset_catalog"] = deployplan.ActionTypeSkip
triggers["dataset_schema"] = deployplan.ActionTypeSkip